Conversation
Ivana-
left a comment
There was a problem hiding this comment.
Good job, but pay attention on some points I marked, and try to code more Clojure-way :)
| ; (if (> i n) | ||
| ; r | ||
| ; (loop [j 0 s nil] | ||
| ; (if (< j n) (recur (+ 1 j) (conj s [i j])) s))))) |
There was a problem hiding this comment.
inc is more idiomatic than (+ 1 .. but it's a style case
| (if (= s1-elem s2-elem) (assoc-in t [i j] (+ i-1j-1 1)) (assoc-in t [i j] (max i-1j ij-1))))) | ||
| t [(vec (repeat (+ 1 (count s1)) 0))] | ||
| r (vec (repeat (+ 1 (count s2)) [0])) | ||
| table (vec (concat t r)) |
There was a problem hiding this comment.
You may use maps instead of vectors for DP, and then you don't need to prefill it by zeros, all getters has variants with defaults (get-in {} [1 2] 33) => 33
| (defn aux [s len pos] | ||
| (cond | ||
| (>= pos len) true | ||
| (= (nth s pos) (nth s (- len pos))) (aux s len (+ 1 pos)) |
There was a problem hiding this comment.
There is no such thing as TCO in Clojure, so recur would be better
| (require '[clojure.string :as str]) | ||
|
|
||
| (def letters | ||
| [\a \b \c \d \e \f \g \h \i \j \k \l \m \n \o \p \q \r \s \t \u \v \w \x \y \z]) |
| (def chars-set (set (map char (range (int \a) (+ (int \a) 26))))) | ||
|
|
||
| (defn is-pangram [test-string] | ||
| (>= (count (reduce (fn [a e] (conj a e)) (set []) (str/lower-case test-string))) 26) |
There was a problem hiding this comment.
(fn [a e] (conj a e)) == conj :)
(set []) == #{} :)
(reduce (fn [a e] (conj a e)) (set []) xxx) == (set xxx) :)))
PS: you'll reduce all the string, even if it's already pangram on it's first 2%
PPS: "0123456789!@#$%^&*()_+|~<>,." will be pangram by your code :)
| (defn normalize-text [text] | ||
| (-> text | ||
| (clojure.string/replace #"\s+|\p{Punct}" "") | ||
| clojure.string/lower-case)) |
There was a problem hiding this comment.
str/lower-case. You already imported qualified ns on line 3 of this file
| (ns otus-02.homework.pangram | ||
| (:require [clojure.string :as string])) | ||
| (:require [clojure.string :as string] | ||
| [clojure.string :as str])) |
| transposed (map (fn [i] (map #(nth % i \space) a)) (range n)) | ||
| res (remove-spaces (apply str (map (fn [elem] (apply str (map str elem))) transposed)))] | ||
| res)) | ||
|
|
There was a problem hiding this comment.
It may work, but.... Looks like you try to use imperative algorithms with mutable state in a case of Clojure, where we prefer immutable data and functional way.
Hope we'll discuss it on Q&A stream
Ivana-
left a comment
There was a problem hiding this comment.
Хорошо, заметен прогресс. Некоторые моменты прокомментировал
| (with-open [file (FileInputStream. file-path) | ||
| buffer (BufferedInputStream. file)] | ||
| (let [file-size (.available file) | ||
| byte-array (byte-array file-size)] |
There was a problem hiding this comment.
Вот так затенять именами переменных имена функций стандартной либы чревато тем, что в динамически типизированном языке при компиляции оно тебе не покажет ошибку, но потом в рантайме свалится, когда ты забудешь и ниже по коду вызовешь эти библиотечные функции по имени. Сто раз уже нарывались на это с name, first и т.п.
| bytes)) | ||
|
|
||
| (defn change-order [[b1 b2 & rest]] | ||
| (if (nil? rest) (conj rest b1 b2) (conj (change-order rest) b1 b2))) |
There was a problem hiding this comment.
Не будет стековерфлоу на больших байтовых массивах?
| (String. (byte-array byte-vector) "UTF-8")) | ||
|
|
||
| (defn parse-content [enc buffer] | ||
| (cond |
|
|
||
| (defmulti decode-frame dispatch-frame) | ||
|
|
||
| (defmethod decode-frame "TALB" [frame] |
| (defmulti decode-frame dispatch-frame) | ||
|
|
||
| (defmethod decode-frame "TALB" [frame] | ||
| (merge frame {:content (str "Album: " (:content frame))})) |
There was a problem hiding this comment.
update frame :content #(str "Album: " %)
| size (decode-synchsafe size-v) | ||
| [flags r3] (split-at 2 r2) | ||
| [enc r4] (split-at 1 r3) | ||
| content (parse-content (first enc) (take (- size 1) r4))] |
| :enc enc | ||
| :content content | ||
| :rest (drop size r3) | ||
| :rest-size (- total-size size 10)})) |
There was a problem hiding this comment.
Достаточно одного раза, побочных эффектов нет :)
| (loop [buffer b | ||
| total size | ||
| frames []] | ||
| (let [{rest :rest rest-size :rest-size content :content id :id enc :enc :as frame} (read-frame buffer total) |
There was a problem hiding this comment.
(let [{:keys [rest rest-size content id enc] :as frame}
Ivana-
left a comment
There was a problem hiding this comment.
Хорошо, к некоторым стилевым моментам придираться не стал :)
|
|
||
| (defn gen [properties] | ||
| (->> (keys properties) | ||
| (reduce (fn [acc k] (assoc acc k (to-spec (get properties k)))) {}) |
There was a problem hiding this comment.
Можно сразу (reduce (fn [acc [k v]] на properties - мапа переводится в сиквенс пар ключ-значение. Это если не выпендриваться с reduce-kv :)
| (def valid-types #{"string" "number" "boolean" "integer"}) | ||
|
|
||
| (defn is-valid-spec [spec] | ||
| (let [fields-object (get-in spec ["properties"]) |
There was a problem hiding this comment.
(get spec "properties"), или (defn is-valid-spec [{:strs [properties] :as spec}]
| (let [fields-object (get-in spec ["properties"]) | ||
| fields (keys fields-object) | ||
| types (map (fn [field] (get-in fields-object [field, "type"])) fields) | ||
| are-types-valid (every? (fn [e] (contains? valid-types e)) types)] |
There was a problem hiding this comment.
(every? valid-types types) - сет имплементирует интерфейс IFn
| (defn render-gen [parsed-spec] | ||
| {:status 200 | ||
| :headers {"Content-Type" "application/json"} | ||
| :body (gen (get-in parsed-spec ["properties"]))}) |
| (defroutes app-routes | ||
| (GET "/" request | ||
| (let [params (:query-params request) | ||
| spec (get-in params ["spec"]) |
There was a problem hiding this comment.
(let [{:strs [spec]} (:query-params request)
| (GET "/" request | ||
| (let [params (:query-params request) | ||
| spec (get-in params ["spec"]) | ||
| parsed-spec (json/parse-string spec)] |
There was a problem hiding this comment.
Если не валится с ошибкой, когда spec == nil то ок
| (def app | ||
| (-> app-routes | ||
| (wrap-reload) | ||
| wrap-params |
There was a problem hiding this comment.
Чем wrap-reload заслужил скобки, а wrap-params нет? :)
No description provided.